fix(hooks): register configured hooks in TUI gateway and serve backends - #111315
wesleymatosdev wants to merge 1 commit into
Conversation
Every long-lived agent runtime must register the user's shell hooks and outbound webhooks at startup. The CLI (_prepare_agent_startup) and the messaging gateway (gateway/run.py) both do; three backends did neither: * tui_gateway.entry.main — 'hermes --tui' stdio backend (and the dashboard chat PTY, which spawns the same entry point) * tui_gateway.ws.handle_ws — the dashboard / desktop WebSocket sidecar * hermes_cli.web_server._lifespan — 'hermes serve' / dashboard backend Net effect: a webhook configured in config.yaml fired in 'hermes --cli' but silently never fired from a TUI, dashboard, or desktop session — including the on_session_end event integration points rely on for completion signaling. Nine open PRs (NousResearch#67084, NousResearch#78805, NousResearch#81409, NousResearch#83997, whole class instead. Add agent/hook_registration.ensure_hooks_registered() — once-per-process guard, shell-hook consent semantics unchanged (flag / env / config opt-in, fail-closed on non-TTY stdin), outbound registration unchanged, fail-soft: a broken hook config never takes down a backend. Wire it into all three backends. The two inline call sites in main.py and gateway/run.py behave identically and can migrate later without behavior change. Regression coverage in tests/tui_gateway/test_hook_registration_parity.py: registration content, explicit-cfg path, once-per-process guard, failure isolation, and all three entry-point wirings (7 tests).
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved profile-scoping, concurrency, plugin-ordering, and slash-worker registration issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR registers configured shell hooks and outbound webhooks across TUI, WebSocket, and serve/dashboard backends.
Changes:
- Adds a shared fail-soft hook registration helper.
- Wires registration into TUI, WS, and web-server startup.
- Adds seven parity and failure-isolation tests.
File summaries
| File | Reviewed changes |
|---|---|
tui_gateway/ws.py |
Adds WebSocket startup wiring for hook registration. |
tui_gateway/server.py |
Adds the registration delegate; profile-specific and slash-worker registration gaps remain. |
tui_gateway/entry.py |
Adds stdio TUI startup wiring. |
tests/tui_gateway/test_hook_registration_parity.py |
Adds registration parity tests; additional profile and worker coverage is needed. |
hermes_cli/web_server.py |
Registers hooks during serve startup. |
agent/hook_registration.py |
Adds shared registration, but requires profile scoping, synchronized completion, and correct plugin-discovery ordering. |
Review details
Suppressed comments (3)
agent/hook_registration.py:58
- Setting
_ensured = Truebeforeload_config()and both registrations finish makes the guard report completion while work is still in progress.handle_wsinvokes this from multiple concurrent WebSocket tasks; a second connection can return here and process its first RPC before the first connection has registered the webhooks. Use an in-progress/completed state (or synchronize on the registration lock) so other callers wait for completion and only skip after registration has finished.
with _ensured_lock:
if _ensured:
return
_ensured = True
agent/hook_registration.py:67
- These callbacks are appended before plugin discovery on the new serve/WS startup path:
_lifespancallsensure_hooks_registered()before agent setup discovers plugins.PluginDispatchMixin.invoke_hookpreserves_hooksinsertion order andpre_tool_calluses the first directive; gateway startup explicitly discovers plugins before config hooks (gateway/run_startup.py:875-910) so plugin policy wins ties. Discover/load the active profile's plugins before registering config callbacks, otherwise a shell block can preempt a plugin policy callback.
from agent import outbound_webhooks, shell_hooks
shell_hooks.register_from_config(cfg, accept_hooks=accept_hooks)
outbound_webhooks.register_from_config(cfg)
tui_gateway/server.py:391
- The slash worker is a separate subprocess with a fresh hook registry. The
hermes serve/dashboard path does not setHERMES_DEFER_AGENT_STARTUP, so itsHermesCLI._init_agent()reaches_prepare_deferred_agent_startup()and returns without registering config hooks; the parent registration cannot cross that process boundary. Agent-backed slash commands therefore still miss configured shell hooks and outbound webhooks. Register the worker at startup using itsprofile_home, or call the helper intui_gateway.slash_worker, and cover this path.
from agent.hook_registration import ensure_hooks_registered
ensure_hooks_registered()
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| _ensured_lock = threading.Lock() | ||
| _ensured = False |
| from agent.hook_registration import ensure_hooks_registered | ||
|
|
||
| ensure_hooks_registered() |
alt-glitch
added
type/bug
Related: open single-path PRs for the same hook-registration gap: #98889 (serve/dashboard), #102151 and #102665 (serve), #78805 (dashboard + TUI slash-worker). This PR proposes one shared helper across all backends; reviewers should pick between the consolidated approach here and the per-path fixes. |
|
exact-head c28866f — void if moved KEEP: shared PICK-ONE vs open spray (same gap, narrower footprint):
Recommendation: land this PR; close or rebase the spray onto it. Soft note: #53894 (profile-keyed session hooks) overlaps CHECK: Copilot called out profile-scoping / concurrency / plugin-ordering — worth a quick pass before merge. Soft file overlap with #111292 on |
|
Reproduced on v0.20.2 (2026.8.16), Linux — same root cause, found independently before I saw this PR. Adding the evidence in case it helps get this merged: 1. 2. The messaging gateway registers the very same hook fine 3. In the desktop panel / TUI path the hook is silently a no-op
4. Not caching / not config staleness Full app restart (wrapper + gateway + dashboard + tui_gateway all brand-new PIDs) → gateway sessions protected, panel/TUI still unprotected. Pure code-path gap. Why this one matters: with |
Problem
Hooks and webhooks configured in
config.yamlfire onhermes --cli(viahermes_cli.main._prepare_agent_startup) and on the messaging gateway (gateway/run.py), but never from the TUI gateway backends:hermes --tui(stdio backend,tui_gateway/entry.py)tui_gateway/ws.py)hermes serve/ dashboard backend (hermes_cli/web_server.py)A user who configures a shell hook or outbound webhook gets silent behavior differences depending on which entry point their session happens to be driven through. This is also why the fix keeps landing piecemeal: #67084, #78805, #81409, #83997, #87116, #92655, #98889 (and #34112 / #41464) each patch one path.
Fix
New
agent/hook_registration.ensure_hooks_registered()— once-per-process guard, never raises:tui_gateway/entry.pymain()andtui_gateway/ws.pyhandle_wsregister via a thintui_gateway/server._register_hooks_from_config()delegatehermes_cli/web_server.py_lifespanregisters at serve startupConsent semantics are unchanged (still owned by
agent.shell_hooks: flag / env / config opt-in, fail-closed on non-TTY stdin — neither entry point prompts on piped stdio). Fail-soft: a broken hook config can't take down a backend. The existing inline registrations in the CLI and gateway behave identically and can migrate to the shared helper later without behavior change.Testing
tests/tui_gateway/test_hook_registration_parity.py— 7 tests covering: once-per-process guard, entry-point wiring (entry / ws / web_server), consent passthrough, and fail-soft behavior.Supersedes the single-path PRs listed above.